fix(fsapp): drop on_final from turn hook so exit_reason no longer finalizes the task (#685) - #760

Summary

_make_task_hook in frontends/fsapp.py previously treated any turn-level exit_reason as completion of the whole Feishu task and invoked the task's on_final callback (_finish) immediately. That callback sets result["sent"] = True, so run_agent could leave its display-queue loop before the queue published its real {"done": ...} item. The card could be marked complete from an intermediate turn response, and final generated-file/attachment handling could run with the wrong text.

The same hook also had a second bug: when a single context contained both exit_reason and summary, the exit_reason branch won, so the per-turn step was not added to the card.

This PR makes the hook summary-only — it only patches the per-turn card step from summary. Finalization is now reachable only via the display-queue done item (and the timeout/stop/exception paths in run_agent, which were unchanged).

Fixes #685.

Root cause

_make_task_hook(card, task_id, on_final) was passed _finish as on_final. The inner hook then fired on_final whenever ctx['exit_reason'] was truthy. Because exit_reason is a per-turn signal (see agent_loop.py:73-92 — exit_reason is set inside the per-turn outcome loop, not the per-task one), an intermediate turn-end would prematurely finalize the queued task.

Reproduction

Issue #685 includes an isolated harness exercising _make_task_hook:

[01_exit_reason_with_response_triggers_on_final]
expect={'on_final': 1, 'step': 0, 'fail': 0}
actual={'on_final': 1, 'step': 0, 'fail': 0}

[05_summary_and_exit_reason_together_exit_reason_wins]
expect={'on_final': 1, 'step': 0, 'fail': 0}
actual={'on_final': 1, 'step': 0, 'fail': 0}
on_final_arg='final text'

After this fix, both on_final calls disappear (finalization belongs to the queue's done item) and the summary is preserved as a card step.

Change

• _make_task_hook(card, task_id, on_final) → _make_task_hook(card, task_id)
• Inner hook drops the exit_reason branch and the on_final(raw) invocation. The summary branch is unchanged.
• Caller in run_agent no longer passes _finish. The display-queue loop's if item and "done" in item: await asyncio.to_thread(_finish, item.get("done", "")) path is the sole _finish entrypoint (alongside timeout/stop/exception, which were untouched).

Net diff: 12 insertions, 8 deletions in frontends/fsapp.py.

Test

tests/test_fsapp_exit_reason_no_finalize.py covers:

1. exit_reason + response → no _finish, no step (the core [fsapp] turn-end exit_reason can finalize a queued task before display queue done #685 case).
2. summary + exit_reason → step is recorded, no _finish.
3. summary alone → step is recorded, no _finish.
4. stale task_id → hook is a no-op.
5. _make_task_hook signature no longer carries on_final.
6. behavioral pin of the old on_final branch — keeps the regression test honest if fsapp.py ever regains an on_final path.

The tests load the live _make_task_hook AST source so they exercise the actual upstream function (not a copy) and can be run with python tests/test_fsapp_exit_reason_no_finalize.py on a fresh clone (no pytest, no node_modules, no Feishu credentials).

Verification

python tests/test_fsapp_exit_reason_no_finalize.py
# PASS test_hook_does_not_finalize_on_exit_reason_with_response
# PASS test_hook_adds_step_when_summary_present_with_exit_reason
# PASS test_hook_adds_step_for_summary_only
# PASS test_hook_is_no_op_for_stale_task_id
# PASS test_hook_signature_no_longer_takes_on_final
# PASS test_old_signature_would_have_finalized_on_exit_reason
# ALL PASS

python -m py_compile frontends/fsapp.py

The same suite was also run against the unfixed source (git checkout main -- frontends/fsapp.py) and correctly reports the missing on_final argument / old signature.

Closes #685